Conversation
|
lib/json/stream/buffer.rb
Outdated
| # Avoid state machine for complete UTF-8. | ||
| if @buffer.empty? | ||
| data.force_encoding(Encoding::UTF_8) | ||
| (+data).force_encoding(Encoding::UTF_8) |
There was a problem hiding this comment.
@kou Yes, this works. (Edited. Sorry, I misunderstood the concern you were raising, initially).
The construct (+data) (or String.new(data)) already returns a dup if the String is frozen. Another dup is unnecessary.
3.4.5 :022 > x = String.new("foo", encoding: 'ASCII-8BIT').freeze;
y = (+x).force_encoding('UTF-8');
z = (+y);
3.4.5 :023 > [x, y, z].map(&:encoding)
=> [#<Encoding:BINARY (ASCII-8BIT)>, #<Encoding:UTF-8>, #<Encoding:UTF-8>]
3.4.5 :024 > [x, y, z].map(&:frozen?)
=> [true, false, false]
3.4.5 :025 > [x, y, z].map(&:object_id)
=> [326840, 326848, 326848]
By the way, this change (to #<<) seems like it's only necessary in test context. I can't imagine real world cases where the append data would be a frozen String literal. I don't love the fact that we mutate our inputs, but that was already happening.
There was a problem hiding this comment.
If data is frozen, (+data).force_encoding(Encoding::UTF_8) does nothing because the (+data).force_encoding(Encoding::UTF_8) result is ignored. The below if data.valid_encoding? check may be done with non UTF-8 encoding.
If data's encoding is always UTF-8, we don't need this (+data).force_encoding(Encoding::UTF_8) entirely.
If data's encoding may not be UTF-8, we need data = (+data).force_encoding(Encoding::UTF_8) or data = +data; data.force_encoding(Encoding::UTF_8).
|
Is this something that could get merged? Just making sure this library stays ahead of the frozen string literal changes. |
Fixes #9